-
Notifications
You must be signed in to change notification settings - Fork 6.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
treewide: Use hyphens instead of underscores for property separation #83352
base: main
Are you sure you want to change the base?
Conversation
I have a lot of work to catch up on right now, like CI checks and other underlying changes (maybe due to a lack of focus). |
f765f9a
to
debb691
Compare
I wrote another PR which adds the devicetree coding style to the contributor guide. |
Call me skeptical (hopeful?), but I think that there is probably a way to a) activate CI checks only when a DT binding has changed, and b) find one binding to change that does not require changing the entire tree all at once. |
@rruuaanng thanks again for taking this on. Just to clarify, you don't need my personal authorization to add my signed-off-by: to any commits that you create, as long as I already added the signed-off-by: myself and opened a pull request with that commit to upstream zephyr. When I added my signed-off-by: and created a PR, I was certifying for legal purposes (see the DCO docs) that I was able to submit the code under the appropriate license (apache 2 or other file-specific license) to the project at the time I opened the PR. You are free to use the code under that license however you want, as long as you comply with the license's terms. You don't need any additional permission from me. The only thing you have to do is make sure that my signed-off-by: lines stay on any commits you submit yourself if they are adapted from my work. |
Oh, I almost forgot. Currently, the binding style checks are skipped when no changes are made to the bindings. Let's wait and see.
Edit If this operation needs to be implemented, I will need to remove the
That is, I did not need to make any changes to edtlib. |
I hope I haven't misunderstood. This is my current commit message:
This commit is based on your work, so I should change it to the following:
But for content I have written myself, I only need to use my own signature:
|
9fcf476
to
27a66b7
Compare
CI throw
Okay, it looks like only the signature issue remains. It seems like there might be a misunderstanding on my part? I might need to keep the original commit message.
Hmmm.... It seems that the development process description isn't very clear on this. I apologize for not understanding it even after reading it multiple times. Does "keep" mean adding two signatures? Like this:
|
@rruuaanng - you should be able to do |
Should I add two signatures? Like this
|
Yes |
27a66b7
to
eb2d68a
Compare
Sure, everything looks good with the CI, and we can make a decision on the proposal. |
"./scripts/migrate_bindings_style.py" = [ | ||
"I001", # https://docs.astral.sh/ruff/rules/unsorted-imports | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably can work around this using ruff
- @pdgendt is fairly familiar with that.
@@ -36,6 +37,28 @@ | |||
import list_boards | |||
import list_hardware | |||
|
|||
sys.path.insert(0, os.path.join(str(Path(__file__).resolve().parents[2]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm surprised that so much needs to be added in this file
|
||
|
||
# bindings base | ||
BINDINGS_PATH = ["dts/bindings/"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably want this to be a list of Path objects
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strings list are convenient for command lines to pass parameters, see line 84
the commits changing the tree dont seem to exist anymore
This is correct -- for commits based on mine that you altered or incorporated here, use both my and your signed-off-by-lines. But for work that is yours alone, use just your signed-off-by. |
This script replaces all underscore(_) separation in a binding with hyphens(-). Signed-off-by: James Roy <[email protected]>
Implement a check in the CI pipeline to enforce that property names in device tree bindings do not contain underscores. Signed-off-by: Martí Bolívar <[email protected]> Signed-off-by: James Roy <[email protected]>
Bindings now use hyphens(-) instead of underscores(_) as property separators. Signed-off-by: James Roy <[email protected]>
eb2d68a
to
fe05263
Compare
Change log: Reorganize the description of the migration document. |
This inherits from proposal #53502, with the main work as shown in the title. For details, see #53506.